Marking s-s for now since the assert message looks bad. This testcase does require the fuzzPriv extension to help with reliable reproduction (https://github.com/MozillaSecurity/domfuzz/tree/master/dom/extension) It may take a moment but this testcase does work reliably. I have tested with the latest m-c debug and ASan debug builds. Assertion failure: isInList(), at /src/obj-firefox/dist/include/mozilla/LinkedList.h:243 #0 mozilla::LinkedListElement<RefPtr<mozilla::dom::Timeout> >::remove() /src/obj-firefox/dist/include/mozilla/LinkedList.h:248:11 #1 nsCycleCollector::CollectWhite() /src/xpcom/base/nsCycleCollector.cpp:3401:26 #2 nsCycleCollector::Collect(ccType, js::SliceBudget&, nsICycleCollectorListener*, bool) /src/xpcom/base/nsCycleCollector.cpp:3769:24 #3 nsCycleCollector_collect(nsICycleCollectorListener*) /src/xpcom/base/nsCycleCollector.cpp:4315:21 #4 nsJSContext::CycleCollectNow(nsICycleCollectorListener*) /src/dom/base/nsJSEnvironment.cpp:1479:3 #5 nsDOMWindowUtils::CycleCollect(nsICycleCollectorListener*) /src/dom/base/nsDOMWindowUtils.cpp:1437:3 #6 NS_InvokeByIndex /src/xpcom/reflect/xptcall/md/unix/xptcinvoke_asm_x86_64_unix.S:129 #7 CallMethodHelper::Call() /src/js/xpconnect/src/XPCWrappedNative.cpp:1315:21 #8 XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) /src/js/xpconnect/src/XPCWrappedNative.cpp:1282:34 #9 XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) /src/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:929:12 #10 js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) /src/js/src/jscntxtinlines.h:291:15 #11 js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:472:16 #12 InternalCall(JSContext*, js::AnyInvokeArgs const&) /src/js/src/vm/Interpreter.cpp:521:12 #13 Interpret(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:3061:18 #14 js::RunScript(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:422:12 #15 js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:494:15 #16 InternalCall(JSContext*, js::AnyInvokeArgs const&) /src/js/src/vm/Interpreter.cpp:521:12 #17 js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /src/js/src/vm/Interpreter.cpp:540:10 #18 JS_CallFunctionValue(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /src/js/src/jsapi.cpp:2973:12 #19 xpc::FunctionForwarder(JSContext*, unsigned int, JS::Value*) /src/js/xpconnect/src/ExportHelpers.cpp:315:18 #20 js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) /src/js/src/jscntxtinlines.h:291:15 #21 js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:472:16 #22 InternalCall(JSContext*, js::AnyInvokeArgs const&) /src/js/src/vm/Interpreter.cpp:521:12 #23 Interpret(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:3061:18 #24 js::RunScript(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:422:12 #25 js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:494:15 #26 InternalCall(JSContext*, js::AnyInvokeArgs const&) /src/js/src/vm/Interpreter.cpp:521:12 #27 js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /src/js/src/vm/Interpreter.cpp:540:10 #28 JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /src/js/src/jsapi.cpp:3032:12 #29 mozilla::dom::EventListener::HandleEvent(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Event&, mozilla::ErrorResult&) /src/obj-firefox/dom/bindings/EventListenerBinding.cpp:47:8 #30 void mozilla::dom::EventListener::HandleEvent<mozilla::dom::EventTarget*>(mozilla::dom::EventTarget* const&, mozilla::dom::Event&, mozilla::ErrorResult&, char const*, mozilla::dom::CallbackObject::ExceptionHandling, JSCompartment*) /src/obj-firefox/dist/include/mozilla/dom/EventListenerBinding.h:65:12 #31 mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, nsIDOMEvent*, mozilla::dom::EventTarget*) /src/dom/events/EventListenerManager.cpp:1115:9 #32 mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*) /src/dom/events/EventListenerManager.cpp:1293:20 #33 mozilla::EventTargetChainItem::HandleEvent(mozilla::EventChainPostVisitor&, mozilla::ELMCreationDetector&) /src/dom/events/EventDispatcher.cpp:313:17 #34 mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) /src/dom/events/EventDispatcher.cpp:462:16 #35 mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) /src/dom/events/EventDispatcher.cpp:826:9 #36 nsDocumentViewer::LoadComplete(nsresult) /src/layout/base/nsDocumentViewer.cpp:1064:7 #37 nsDocShell::EndPageLoad(nsIWebProgress*, nsIChannel*, nsresult) /src/docshell/base/nsDocShell.cpp:7765:21 #38 nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) /src/docshell/base/nsDocShell.cpp:7563:7 #39 non-virtual thunk to nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) /src/docshell/base/nsDocShell.cpp:7460:13 #40 nsDocLoader::DoFireOnStateChange(nsIWebProgress*, nsIRequest*, int&, nsresult) /src/uriloader/base/nsDocLoader.cpp:1321:3 #41 nsDocLoader::doStopDocumentLoad(nsIRequest*, nsresult) /src/uriloader/base/nsDocLoader.cpp:862:14 #42 nsDocLoader::DocLoaderIsEmpty(bool) /src/uriloader/base/nsDocLoader.cpp:751:9 #43 nsDocLoader::DocLoaderIsEmpty(bool) /src/uriloader/base/nsDocLoader.cpp:754:19 #44 nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) /src/uriloader/base/nsDocLoader.cpp:633:5 #45 non-virtual thunk to nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) /src/uriloader/base/nsDocLoader.cpp:489:14 #46 mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, nsresult) /src/netwerk/base/nsLoadGroup.cpp:629:28 #47 mozilla::net::nsLoadGroup::Cancel(nsresult) /src/netwerk/base/nsLoadGroup.cpp:258:15 #48 nsDocLoader::Stop() /src/uriloader/base/nsDocLoader.cpp:247:22 #49 nsDocShell::Stop(unsigned int) /src/docshell/base/nsDocShell.cpp:5659:5 #50 nsDocShell::InternalLoad(nsIURI*, nsIURI*, mozilla::Maybe<nsCOMPtr<nsIURI> > const&, bool, nsIURI*, unsigned int, nsIPrincipal*, nsIPrincipal*, unsigned int, nsTSubstring<char16_t> const&, char const*, nsTSubstring<char16_t> const&, nsIInputStream*, long, nsIInputStream*, unsigned int, nsISHEntry*, bool, nsTSubstring<char16_t> const&, nsIDocShell*, nsIURI*, bool, nsIDocShell**, nsIRequest**) /src/docshell/base/nsDocShell.cpp:10781:12 #51 nsDocShell::LoadURI(nsIURI*, nsIDocShellLoadInfo*, unsigned int, bool) /src/docshell/base/nsDocShell.cpp:1612:10 #52 nsFrameLoader::ReallyStartLoadingInternal() /src/dom/base/nsFrameLoader.cpp:998:19 #53 nsFrameLoader::ReallyStartLoading() /src/dom/base/nsFrameLoader.cpp:879:17 #54 nsDocument::MaybeInitializeFinalizeFrameLoaders() /src/dom/base/nsDocument.cpp:7602:13 #55 nsDocument::EndUpdate(unsigned int) /src/dom/base/nsDocument.cpp:5427:3 #56 nsHTMLDocument::EndUpdate(unsigned int) /src/dom/html/nsHTMLDocument.cpp:2522:15 #57 mozAutoDocUpdate::~mozAutoDocUpdate() /src/dom/base/mozAutoDocUpdate.h:40:18 #58 mozilla::dom::Element::SetAttr(int, nsAtom*, nsAtom*, nsTSubstring<char16_t> const&, nsIPrincipal*, bool) /src/dom/base/Element.cpp:2623:1 #59 nsIContent::SetAttr(int, nsAtom*, nsTSubstring<char16_t> const&, nsIPrincipal*, bool) /src/dom/base/nsIContent.h:385:12 #60 mozilla::dom::Element::SetAttr(nsAtom*, nsTSubstring<char16_t> const&, nsIPrincipal&, mozilla::ErrorResult&) /src/obj-firefox/dist/include/mozilla/dom/Element.h:1417:26 #61 mozilla::dom::HTMLIFrameElementBinding::set_src(JSContext*, JS::Handle<JSObject*>, mozilla::dom::HTMLIFrameElement*, JSJitSetterCallArgs) /src/obj-firefox/dom/bindings/HTMLIFrameElementBinding.cpp:74:9 #62 mozilla::dom::GenericBindingSetter(JSContext*, unsigned int, JS::Value*) /src/dom/bindings/BindingUtils.cpp:3001:8 #63 js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) /src/js/src/jscntxtinlines.h:291:15 #64 js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:472:16 #65 InternalCall(JSContext*, js::AnyInvokeArgs const&) /src/js/src/vm/Interpreter.cpp:521:12 #66 js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /src/js/src/vm/Interpreter.cpp:540:10 #67 js::CallSetter(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::Handle<JS::Value>) /src/js/src/vm/Interpreter.cpp:669:12 #68 SetExistingProperty(JSContext*, JS::Handle<js::NativeObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::Handle<js::NativeObject*>, JS::Handle<JS::PropertyResult>, JS::ObjectOpResult&) /src/js/src/vm/NativeObject.cpp:2730:10 #69 bool js::NativeSetProperty<(js::QualifiedBool)1>(JSContext*, JS::Handle<js::NativeObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::ObjectOpResult&) /src/js/src/vm/NativeObject.cpp:2758:20 #70 js::SetProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::ObjectOpResult&) /src/js/src/vm/NativeObject.h:1621:12 #71 SetPropertyOperation(JSContext*, JSOp, JS::Handle<JS::Value>, JS::Handle<jsid>, JS::Handle<JS::Value>) /src/js/src/vm/Interpreter.cpp:269:12 #72 Interpret(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:2858:10 #73 js::RunScript(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:422:12 #74 js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:494:15 #75 InternalCall(JSContext*, js::AnyInvokeArgs const&) /src/js/src/vm/Interpreter.cpp:521:12 #76 js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /src/js/src/vm/Interpreter.cpp:540:10 #77 JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /src/js/src/jsapi.cpp:3032:12 #78 mozilla::dom::EventHandlerNonNull::Call(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Event&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&) /src/obj-firefox/dom/bindings/EventHandlerBinding.cpp:260:37 #79 void mozilla::dom::EventHandlerNonNull::Call<nsISupports*>(nsISupports* const&, mozilla::dom::Event&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&, char const*, mozilla::dom::CallbackObject::ExceptionHandling, JSCompartment*) /src/obj-firefox/dist/include/mozilla/dom/EventHandlerBinding.h:362:12 #80 mozilla::JSEventHandler::HandleEvent(nsIDOMEvent*) /src/dom/events/JSEventHandler.cpp:215:12 #81 mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, nsIDOMEvent*, mozilla::dom::EventTarget*) /src/dom/events/EventListenerManager.cpp:1118:51 #82 mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*) /src/dom/events/EventListenerManager.cpp:1293:20 #83 mozilla::EventTargetChainItem::HandleEvent(mozilla::EventChainPostVisitor&, mozilla::ELMCreationDetector&) /src/dom/events/EventDispatcher.cpp:313:17 #84 mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) /src/dom/events/EventDispatcher.cpp:462:16 #85 mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) /src/dom/events/EventDispatcher.cpp:826:9 #86 (anonymous namespace)::AsyncTimeEventRunner::Run() /src/dom/smil/nsSMILTimedElement.cpp:115:14 #87 nsThread::ProcessNextEvent(bool, bool*) /src/xpcom/threads/nsThread.cpp:1037:14 #88 NS_ProcessNextEvent(nsIThread*, bool) /src/xpcom/threads/nsThreadUtils.cpp:513:10 #89 mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /src/ipc/glue/MessagePump.cpp:97:21 #90 MessageLoop::RunInternal() /src/ipc/chromium/src/base/message_loop.cc:326:10 #91 MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:299:3 #92 nsBaseAppShell::Run() /src/widget/nsBaseAppShell.cpp:158:27 #93 nsAppStartup::Run() /src/toolkit/components/startup/nsAppStartup.cpp:288:30 #94 XREMain::XRE_mainRun() /src/toolkit/xre/nsAppRunner.cpp:4664:22 #95 XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /src/toolkit/xre/nsAppRunner.cpp:4826:8 #96 XRE_main(int, char**, mozilla::BootstrapConfig const&) /src/toolkit/xre/nsAppRunner.cpp:4921:21 #97 do_main(int, char**, char**) /src/browser/app/nsBrowserApp.cpp:231:22 #98 main /src/browser/app/nsBrowserApp.cpp:304:16 #99 __libc_start_main /build/glibc-bfm8X4/glibc-2.23/csu/../csu/libc-start.c:291 #100 _start (/home/user/workspace/browsers/m-c-1510166834-asan-debug/firefox+0x41ebe4)

The stack doesn't quite show it, but from the test case, I'm guessing this is related to requestIdleCallback.

Can you take a look, Andreas? Thanks.

This definitely has something to do with rIC and the TimeoutManager, I have it in rr so I'm making progress.

The problem is that we call TimeoutManager::ClearTimeout when unlinking, which will remove Timeout objects from the lists in TimeoutManager, but Timeout expects to remove itself when unlinking (due to it being LinkedListElement<RefPtr<Timeout>>). By making IdleRequest a LinkedListElement<RefPtr<IdleRequest>> as well we can simplify the add/release step for IdleRequest while we at the same time can entirely skip removing timeouts for IdleRequest by also making IdleRequests remove themselves. In the end though, what fixes this issue is that DisableIdleCallbackRequests isn't called inside of NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(nsGlobalWindow). Another possible solution is to add an isInList check in NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(Timeout).

Comment on attachment 8927287 [details] [diff] [review] 0001-Bug-1415770-Simplify-handling-of-IdleRequest-list.patch [Security approval request comment] How easily could an exploit be constructed based on the patch? Not sure, I'd like someone with knowledge of the CycleCollector to check my findings. The error we get is a double remove() on an LinkedListElement<ReftPtr<T>>, which implies a double release of a ref-counted object involved in cc. Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? The patch solves the issue by tidying up IdleRequest, by making IdleRequest also be a LinkedListElement<ReftPtr<T>>, and by doing so making it possible to sneak in the removal of DisableIdleCallbackRequests that is one of the instance where a Timeout is removed. Just adding: if (tmp->isInList()) { tmp->remove() } in dom/base/Timeout.cpp would be more suspect. Which older supported branches are affected by this flaw? I think this gets introduced in Bug 1363829, so firefox55, firefox56, firefox57. If not all supported branches, which bug introduced the flaw? Bug 1363829 Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? No, but I think that they'll merge pretty cleanly. How likely is this patch to cause regressions; how much testing does it need? IdleReqeust manually handles AddRef/Release, and this should generally do the same thing, so it should be fairly safe. Again, someone with CC knowledge would be nice to get feedback from.

I think an extra release can cause the refcount to do all sorts of bad things, so I'm going to mark it sec-high.

Track 58+ as sec-high.

Sec-approval+ for checkin on November 28, two weeks into the new cycle. Please do not check in before that date. We'll want this on beta so a patch should be made and nominated at that time as well.

Had to rebase the patch due to the nsGlobalWindow split. Also added the test for checking if a timeout/idle request is in the list before removing it, this after talking to Olli about it. I think it would be good if Olli could re-review the patch due to this. Due I need to get another sec-approval because of this change?

Added ni for question about sec-approval.

(In reply to Andreas Farre [:farre] from comment #10) > Due I need to get another sec-approval because of this change? No.

Comment on attachment 8931377 [details] [diff] [review] 0001-Bug-1415770-Simplify-handling-of-IdleRequest-list.-r.patch >@@ -1351,17 +1341,16 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(nsGlobalWindowInner) > NS_IMPL_CYCLE_COLLECTION_UNLINK(mMozSelfSupport) > NS_IMPL_CYCLE_COLLECTION_UNLINK(mIntlUtils) > > NS_IMPL_CYCLE_COLLECTION_UNLINK(mDocShell) > > tmp->UnlinkHostObjectURIs(); > > NS_IMPL_CYCLE_COLLECTION_UNLINK(mIdleRequestExecutor) >- tmp->DisableIdleCallbackRequests(); So now we don't unlink mIdleRequestCallbacks at all, but we do traverse them, and rely on IdleRequests unlink to remove from the list. I guess that works too, but please add a comment here how that all works.

(In reply to Olli Pettay [:smaug] from comment #13) > Comment on attachment 8931377 [details] [diff] [review] > 0001-Bug-1415770-Simplify-handling-of-IdleRequest-list.-r.patch > > > >@@ -1351,17 +1341,16 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(nsGlobalWindowInner) > > NS_IMPL_CYCLE_COLLECTION_UNLINK(mMozSelfSupport) > > NS_IMPL_CYCLE_COLLECTION_UNLINK(mIntlUtils) > > > > NS_IMPL_CYCLE_COLLECTION_UNLINK(mDocShell) > > > > tmp->UnlinkHostObjectURIs(); > > > > NS_IMPL_CYCLE_COLLECTION_UNLINK(mIdleRequestExecutor) > >- tmp->DisableIdleCallbackRequests(); > So now we don't unlink mIdleRequestCallbacks at all, but we do traverse > them, and rely on IdleRequests unlink to remove from the list. > I guess that works too, but please add a comment here how that all works. Right, I'll make that happen. The reason for it being that way is that it's the same for Timeouts (and also the reason for this bug), so should I add the comment for the timeouts or is that pointing out the security issue too much?

Just explain in inner window's unlink how timeouts and idle stuff get unlinked

Added comments, and another rebase.

https://hg.mozilla.org/integration/mozilla-inbound/rev/a6f2cfeca8d26f073ad9c599f0ba8520b377068f I assume this'll need a rebased patch for Beta due to the Inner/Outer window split, so please attach that and request Beta approval when you get a chance.

Approval Request Comment [Feature/Bug causing the regression]: Bug 1363829 [User impact if declined]: Security issue [Is this code covered by automated tests?]: No, because of security reasons [Has the fix been verified in Nightly?]: Fix landed 2017-11-29 on Nightly. [Needs manual test from QE? If yes, steps to reproduce]: Manual tests are hard, since a fairly aggressive CC pressure is needed (I've been using a CycleCollect method exposed to JS via Window to achieve this) and this isn't normally present. [List of other uplifts needed for the feature/fix]: None. [Is the change risky?]: After we added the isInList check in the unlink steps it definitely became less risky. [Why is the change risky/not risky?]: See above [String changes made/needed]: None. [Other] Fortunately the patch for beta is pretty similar to the original patch before rebase due to nsGlobalWindowInner, with the exception that the new patch has the isInList() check and comments in the unlink step, both of which has been reviewed in later patches, which makes me consider the beta patch to be already reviewed by Olli, transitively :)

Comment on attachment 8932873 [details] [diff] [review] 0001-Bug-1415770-Simplify-handling-of-IdleRequest-beta.-r.patch Fix a sec-high. Beta58+.